Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CLAUDE.md-67-72 (1)

67-72: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove policy exceptions that contradict the repository docs-location rule.

This new exception list allows Markdown files outside server/docs/, which conflicts with the enforced documentation policy and creates ambiguous standards.

As per coding guidelines: "**/*.md: Place all documentation files exclusively under server/docs/ - no .md documentation files in root, client/, or other directories".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CLAUDE.md` around lines 67 - 72, The "الاستثناءات الوحيدة" section in
CLAUDE.md currently exempts `CLAUDE.md` (and other Markdown files) from the
repository docs-location rule, which conflicts with the enforced policy
requiring all `**/*.md` docs to live under server/docs/; to fix it, remove or
modify the exception lines that state "`CLAUDE.md` يبقى في `server/CLAUDE.md`"
and any related bullets so the "الاستثناءات الوحيدة" section no longer permits
Markdown files outside server/docs/, and update CLAUDE.md to reference the
global documentation rule instead (locate and edit the "الاستثناءات الوحيدة"
header in CLAUDE.md to align its text with the repository's docs-location
policy).

__tests__/unit/Services/QueueService.test.ts-44-44 (1)

44-44: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the no-op shared-client touch.

Line 44 currently references getTestRedis without invoking it, so the intended side-effect does not happen.

Suggested patch

-    void getTestRedis;
+    void getTestRedis();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/unit/Services/QueueService.test.ts` at line 44, The test currently
references getTestRedis as a no-op symbol (void getTestRedis;) so its
side-effect never runs; replace that reference with an actual invocation — call
getTestRedis() and await it if it returns a Promise (e.g., await getTestRedis()
or void await getTestRedis()); ensure the invocation is placed where the
side-effect was intended in QueueService.test.ts so the shared-client touch
occurs.

__tests__/unit/Services/QueueService.test.ts-127-129 (1)

127-129: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align slow-test job id expectation with the deterministic format.

Line 128 expects '9999', but Line 74 in the same suite asserts Enqueue returns dep-<id>. This inconsistency can fail the opt-in slow test.

Suggested patch

-    expect(jobId).toBe('9999');
+    expect(jobId).toBe('dep-9999');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/unit/Services/QueueService.test.ts` around lines 127 - 129, The
test is expecting the wrong job id format; update the assertion in
QueueService.test.ts to match the deterministic "dep-<id>" format used elsewhere
(e.g., change the expected value from '9999' to 'dep-9999' for the call to
Enqueue), ensuring the test that calls Enqueue(9999, PROJECT_ID, 0) checks for
'dep-9999' so it aligns with the other assertion at Line 74 that Enqueue returns
dep-<id>.

docs/CHANGELOG.md-102-103 (1)

102-103: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix inconsistent migration guide path reference.

The text points to server/docs/migration-v2-to-v3.md, but this PR introduces docs/migration-v2-to-v3.md. Align the referenced path to prevent operator confusion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/CHANGELOG.md` around lines 102 - 103, Update the inconsistent
documentation path in CHANGELOG.md by replacing the incorrect reference string
`server/docs/migration-v2-to-v3.md` with the correct
`docs/migration-v2-to-v3.md` so the line referencing the migration guide (and
the F-003 envVars migration helper SQL) points to the new file location.

docs/ROADMAP.md-45-45 (1)

45-45: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language tags to fenced code blocks to satisfy markdown linting.

Both fenced blocks are missing language identifiers (MD040), which will keep docs lint noisy.

Suggested fix

-```
+```text
 MAJOR.MINOR.PATCH
    |     |     |
    |     |     └─ Bug fixes فقط (v3.0.1, v3.0.2, ...)
    |     └─ ميزات جديدة backward-compatible (v3.0 → v3.1)
    └─ كسر backward compatibility / إعادة هيكلة كبرى (v3 → v4)

- +text
2026:
├── May ──→ v2.1.x (current) - maintenance
...
└── Q3+ → v5.0 GA — Cloud Native (Multi-Region + Distributed + Plugin Marketplace)

Also applies to: 72-72

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/ROADMAP.md` at line 45, Two fenced code blocks in ROADMAP.md are missing
language identifiers (MD040); update the opening fences for the two blocks that
contain the version diagram (the block starting with "MAJOR.MINOR.PATCH...") and
the 2026 roadmap block (the block starting with "2026:") to include a language
tag (e.g., change ``` to ```text) so markdown linting stops flagging them;
locate the fences around those snippets and add the language token to the
opening backticks.

docs/migration-v2-to-v3.md-25-25 (1)

25-25: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the docker-compose link path (likely broken).

From docs/migration-v2-to-v3.md, ../../docker-compose.yml traverses too far up. Use the correct relative path from the docs directory.

Suggested fix

-- **Redis 7+** reachable from the Deploy Center host. Docker Compose recipe
-  bundled — see [docker-compose.yml](../../docker-compose.yml).
+- **Redis 7+** reachable from the Deploy Center host. Docker Compose recipe
+  bundled — see [docker-compose.yml](../docker-compose.yml).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/migration-v2-to-v3.md` at line 25, The markdown link "bundled — see
[docker-compose.yml](../../docker-compose.yml)" points too far up; update the
link target to the correct relative path from the docs file (replace
"../../docker-compose.yml" with "../docker-compose.yml") so the
"[docker-compose.yml]" link resolves to the repository root docker-compose file.

docs/release-engineering.md-5-6 (1)

5-6: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix likely broken relative links in source references.

From docs/release-engineering.md, ../../.specify/... and ../../specs/... likely resolve outside the repository root. These should probably be one level up (for example ../.specify/..., ../specs/...) based on actual repo layout.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/release-engineering.md` around lines 5 - 6, The relative links in
docs/release-engineering.md use ../../ (e.g.,
../../.specify/memory/constitution.md and ../../specs/001-v3-foundation/spec.md)
which likely point outside the repo; update these references to the correct
repo-relative paths (for example ../.specify/memory/constitution.md and
../specs/001-v3-foundation/spec.md) so they resolve inside the repository root,
verify by opening the links locally or running the link-checker, and adjust any
other nearby references using the same ../../ pattern.

docs/versions/v3.1-remote-targets.md-54-54 (1)

54-54: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Specify a language on the fenced API block.

The code fence starting at Line 54 is missing a language tag (MD040). Add one (text is fine if it’s plain route listing).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/versions/v3.1-remote-targets.md` at line 54, The fenced code block in
docs/versions/v3.1-remote-targets.md (the triple-backtick block around the route
listing) is missing a language tag and triggers MD040; add a language identifier
by changing the opening fence from ``` to ```text (or another appropriate
language) so the Markdown linter recognizes the block type.

docs/versions/v3.0-foundation.md-82-82 (1)

82-82: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add language identifiers to fenced code blocks to satisfy markdown lint.

These fenced blocks are missing language tags (MD040). Please annotate them (for example text, bash, json, typescript) so markdownlint passes consistently.

Also applies to: 97-97, 149-149, 212-212, 269-269, 393-393

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/versions/v3.0-foundation.md` at line 82, Several fenced code blocks in
docs/versions/v3.0-foundation.md are missing language identifiers (triple
backticks with no language) causing markdownlint MD040 failures; update each ```
block at the mentioned occurrences to include an appropriate language tag (e.g.,
```text, ```bash, ```json, ```typescript) so the fences are annotated
consistently and linting passes—search for the bare ``` tokens in that file and
replace them with the correct annotated fences.

docs/versions/v5.0-cloud-native.md-104-115 (1)

104-115: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced code block.

The code fence should be tagged (for example, text) to satisfy markdown linting.

Proposed fix

-```
+```text
 Current (v4.x):
   Single Node.js server → Redis → MySQL → SSH/SFTP to targets
@@
-```
+```

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/versions/v5.0-cloud-native.md` around lines 104 - 115, Add a language
identifier to the fenced code block that begins with the lines "Current (v4.x):
Single Node.js server → Redis → MySQL → SSH/SFTP to targets" and the "v5.x:"
architecture diagram; change the opening triple backticks to include a language
token such as text (e.g., ```text) so the block is properly tagged for markdown
linting while leaving the block contents unchanged.

src/Controllers/NotificationChannelController.ts-59-62 (1)

59-62: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate providerId query before calling the service.

parseInt can produce NaN, which currently gets passed through.

Suggested fix

       const providerId = req.query.providerId
         ? parseInt(String(req.query.providerId), 10)
         : undefined;
+      if (providerId !== undefined && (Number.isNaN(providerId) || providerId <= 0)) {
+        ResponseHelper.ValidationError(res, 'Invalid providerId');
+        return;
+      }
       const items = await this.Service.List(providerId);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/NotificationChannelController.ts` around lines 59 - 62, The
code extracts providerId from req.query and uses parseInt which can produce NaN
and is then passed to this.Service.List; change the logic in
NotificationChannelController so after computing providerId (from
req.query.providerId via parseInt), validate it with Number.isFinite or
Number.isInteger (or !Number.isNaN) and only pass a real number to
this.Service.List, otherwise set providerId to undefined or return a 400 for
invalid input; update the variable handling around providerId and the call to
this.Service.List accordingly.

src/Controllers/WorkspaceController.ts-35-43 (1)

35-43: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject malformed route IDs in parseId (WorkspaceController)

• parseInt() accepts numeric prefixes (e.g., 42abc → 42), so the current check can validate an invalid route ID and operate on the wrong workspace/project.
• Handler properties List/Create/Update/Delete/AssignProjectWorkspace are PascalCase and conflict with the camelCase naming guideline.

Suggested fix

 function parseId(req: Request, res: Response, name = 'id'): number | null {
   const raw = req.params[name];
-  const id = raw ? parseInt(raw, 10) : NaN;
-  if (Number.isNaN(id) || id <= 0) {
+  if (!raw || !/^[1-9]\d*$/.test(raw)) {
     ResponseHelper.ValidationError(res, `Invalid ${name}`);
     return null;
   }
-  return id;
+  return Number(raw);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/WorkspaceController.ts` around lines 35 - 43, The parseId
function currently accepts strings like "42abc" because parseInt tolerates
numeric prefixes; update parseId (function parseId) to validate req.params[name]
against a strict digits-only pattern (e.g., /^\d+$/) before converting to a
number and return a ValidationError if it fails, then parseInt/Number(...) and
verify >0; also rename the exported handler properties List, Create, Update,
Delete, AssignProjectWorkspace to camelCase (list, create, update, delete,
assignProjectWorkspace) to follow the project's naming convention and update any
references accordingly.

src/Controllers/ProjectTemplateController.ts-41-47 (1)

41-47: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject malformed :id values (numeric prefixes) in parseId().

parseId() uses parseInt(req.params.id!, 10) and only checks NaN/<= 0, so inputs like "123abc" get treated as 123 and won’t hit ResponseHelper.ValidationError (400). Validate the entire :id string as a positive integer before converting.

Suggested fix

 function parseId(req: Request, res: Response): number | null {
-  const id = parseInt(req.params.id!, 10);
-  if (Number.isNaN(id) || id <= 0) {
+  const rawId = req.params.id;
+  if (!rawId || !/^[1-9]\d*$/.test(rawId)) {
     ResponseHelper.ValidationError(res, 'Invalid id');
     return null;
   }
-  return id;
+  return Number(rawId);
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/ProjectTemplateController.ts` around lines 41 - 47, parseId
currently uses parseInt(req.params.id!, 10) which accepts inputs like "123abc";
change parseId to first validate the raw req.params.id string against a strict
positive-integer pattern (e.g. /^[1-9]\d*$/) and call
ResponseHelper.ValidationError(res, 'Invalid id') returning null if the string
does not match, otherwise safely convert to number (parseInt or Number) and
return it; update references to parseId to preserve behavior.

src/Utils/ResponseHelper.ts-99-110 (1)

99-110: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use camelCase for ResponseHelper static helper method names

src/Utils/ResponseHelper.ts defines public static ServiceUnavailable, Conflict, and UnprocessableEntity in PascalCase, but the repo guideline for **/*.{ts,tsx} requires camelCase for function names. Rename these helpers (e.g., serviceUnavailable, conflict, unprocessableEntity) and update call sites:

• src/Middleware/QueueReadyMiddleware.ts (ResponseHelper.ServiceUnavailable(...))
• src/Controllers/ProjectTemplateController.ts (ResponseHelper.UnprocessableEntity(...))
• src/Controllers/DeploymentController.ts (ResponseHelper.Conflict(...), ResponseHelper.UnprocessableEntity(...))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Utils/ResponseHelper.ts` around lines 99 - 110, Three static helper
methods on ResponseHelper are using PascalCase; rename ServiceUnavailable ->
serviceUnavailable, Conflict -> conflict, and UnprocessableEntity ->
unprocessableEntity in the ResponseHelper class (preserve method signatures and
return types) and update all call sites to the new camelCase names: change
ResponseHelper.ServiceUnavailable(...) in QueueReadyMiddleware to
ResponseHelper.serviceUnavailable(...), ResponseHelper.UnprocessableEntity(...)
in ProjectTemplateController to ResponseHelper.unprocessableEntity(...), and
ResponseHelper.Conflict(...) / ResponseHelper.UnprocessableEntity(...) in
DeploymentController to ResponseHelper.conflict(...) /
ResponseHelper.unprocessableEntity(...); ensure any exports/typings remain valid
after the rename.

__tests__/integration/Auth.test.ts (1)

102-106: ⚡ Quick win

Assert refresh_token clearing on logout too.

This flow only verifies access_token is cleared. Please also assert refresh_token is cleared so logout regression coverage includes both auth cookies.

✅ Suggested assertion addition

     const clears = logout.headers['set-cookie'];
     const cookieStr = Array.isArray(clears) ? clears.join(';') : String(clears);
     // Expect access_token and refresh_token to be cleared (Max-Age=0 or expires past).
     expect(cookieStr).toMatch(/access_token=;|access_token=.*(Max-Age=0|Expires=Thu, 01 Jan 1970)/i);
+    expect(cookieStr).toMatch(/refresh_token=;|refresh_token=.*(Max-Age=0|Expires=Thu, 01 Jan 1970)/i);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/integration/Auth.test.ts` around lines 102 - 106, The logout test
currently only asserts that access_token is cleared; extend it to also assert
that refresh_token is cleared by inspecting the same cookie string (variables
logout, clears, cookieStr) and adding an expectation mirroring the access_token
check (matching refresh_token with either empty value or Max-Age=0/Expires past)
so both auth cookies are validated on logout.

__tests__/integration/Deployments.test.ts (1)

98-110: ⚡ Quick win

Add the retry happy-path integration case.

The suite currently only checks retry rejection on Success. Adding a Failed -> retry -> 201 case would lock in the core retry contract and queue handoff behavior.

🧪 Suggested test addition

+  it('POST /api/deployments/:id/retry on Failed → 201 with new row', async () => {
+    if (!dbUp) return;
+    const admin = await makeUser({ Role: 'Admin' });
+    const project = await makeProject({ CreatedBy: admin.Id });
+    const failed = await makeDeployment({
+      ProjectId: project.Id,
+      Status: EDeploymentStatus.Failed as never,
+    });
+
+    const res = await request(app)
+      .post(`/api/deployments/${failed.Id}/retry`)
+      .set(authHeader(admin));
+
+    expect(res.status).toBe(201);
+    expect(res.body.Data.Deployment.Id).toBeGreaterThan(failed.Id);
+  });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/integration/Deployments.test.ts` around lines 98 - 110, Add a new
integration test in __tests__/integration/Deployments.test.ts that creates an
Admin user and a Project, then creates a deployment with Status set to
EDeploymentStatus.Failed using makeDeployment, calls POST
/api/deployments/${failed.Id}/retry with authHeader(admin), and assert the
response status is 201; additionally verify the retry handoff by asserting the
deployment was enqueued/updated (e.g., check the database record via the same
model used elsewhere or assert the queue/job-mock was called) to ensure the
retry path and queue handoff behavior are exercised.

__tests__/integration/SocketLogStream.test.ts (1)

33-37: 🏗️ Heavy lift

This test bypasses the app’s socket wiring path.

Because it emits directly from an ad-hoc IoServer, regressions in SocketService/server integration won’t be detected by this suite.

Also applies to: 67-70

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/integration/SocketLogStream.test.ts` around lines 33 - 37, The test
is creating an ad-hoc IoServer (IoServer) and emitting directly, bypassing the
app's socket wiring and not exercising SocketService/server integration; update
the test to boot the actual app/socket wiring instead of new IoServer — for
example, start or import the real server/socket initializer used in production
(the code path that registers SocketService handlers) and obtain its
httpServer/port to connect clients, then emit through that real server so
SocketService integration is exercised; make the same change for the second
occurrence around lines 67-70. Ensure you reference and use the app's socket
initialization function or SocketService bootstrap rather than constructing a
raw IoServer in the test.

__tests__/integration/Rollback.test.ts (1)

74-198: ⚡ Quick win

Add the missing 403 authorization rollback case.

This suite validates rollback state rules well, but it does not currently assert that an unrelated developer is blocked from calling rollback (the access-control path).

🔐 Suggested test addition

+  it('403: unrelated developer cannot rollback another project deployment', async () => {
+    if (!dbUp) return;
+    const admin = await makeUser({ Role: 'Admin' });
+    const developer = await makeUser({ Role: 'Developer' });
+    const project = await makeProject({ CreatedBy: admin.Id });
+    const failed = await makeDeployment({
+      ProjectId: project.Id,
+      Status: EDeploymentStatus.Failed as never,
+      CommitHash: 'denied123',
+    });
+
+    const res = await request(app)
+      .post(`/api/deployments/${failed.Id}/rollback`)
+      .set(authHeader(developer));
+
+    expect(res.status).toBe(403);
+  });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/integration/Rollback.test.ts` around lines 74 - 198, Add a new test
case to assert a 403 when a non-admin developer attempts rollback: create a
Developer user via makeUser({ Role: 'Developer' }), a project (owned by some
other user or admin) and a failed deployment via makeDeployment(...) with
EDeploymentStatus.Failed, then POST to /api/deployments/${failed.Id}/rollback
with authHeader(developer) and expect res.status toBe(403); also assert no
AuditLog entry exists for EAuditAction.DeploymentRolledBack and that
QueueService.GetInstance().Enqueue was not called for the failed/new deployment
ID. Use existing helpers (makeUser, makeProject, makeDeployment, authHeader,
AuditLog, QueueService) and the same patterns as the other tests.

__tests__/unit/Services/QueueService.test.ts (1)

19-19: ⚡ Quick win

Use camelCase for the project id variable.

PROJECT_ID should be camelCase to match repository TypeScript naming rules.

Suggested patch

-const PROJECT_ID = 9001;
+const projectId = 9001;
@@
-    const jobId = await QueueService.GetInstance().Enqueue(1, PROJECT_ID, 0);
+    const jobId = await QueueService.GetInstance().Enqueue(1, projectId, 0);
@@
-    await QueueService.GetInstance().Enqueue(2, PROJECT_ID, 0);
-    await QueueService.GetInstance().Enqueue(3, PROJECT_ID, 0);
-    const len = await QueueService.GetInstance().GetQueueLength(PROJECT_ID);
+    await QueueService.GetInstance().Enqueue(2, projectId, 0);
+    await QueueService.GetInstance().Enqueue(3, projectId, 0);
+    const len = await QueueService.GetInstance().GetQueueLength(projectId);
@@
-    const before = await QueueService.GetInstance().GetQueueLength(PROJECT_ID);
-    const cancelled = await QueueService.GetInstance().CancelPendingDeployments(PROJECT_ID);
+    const before = await QueueService.GetInstance().GetQueueLength(projectId);
+    const cancelled = await QueueService.GetInstance().CancelPendingDeployments(projectId);
@@
-    const jobId = await qs.Enqueue(9999, PROJECT_ID, 0);
+    const jobId = await qs.Enqueue(9999, projectId, 0);

As per coding guidelines: "Use camelCase for function names and variable names in TypeScript".

Also applies to: 73-74, 86-89, 94-95, 127-127

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/unit/Services/QueueService.test.ts` at line 19, Rename the all-caps
constant PROJECT_ID to camelCase (e.g., projectId) in
__tests__/unit/Services/QueueService.test.ts and update every reference to it in
the test (including where it's declared and used inside tests like any setup or
assertions); ensure any other uppercase test constants mentioned in the file
follow the same camelCase convention (the comment also flagged other occurrences
around the file) and run the tests to confirm no broken references remain.

src/Database/MigrationRunner.ts (1)

201-226: ⚡ Quick win

Keep the new public API in camelCase.

Since this is new surface area, I’d avoid extending the PascalCase method pattern here. getStatus() matches the repository’s TypeScript naming rule without forcing a broader cleanup in this file.

As per coding guidelines, **/*.{ts,tsx}: Use camelCase for function names and variable names in TypeScript.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Database/MigrationRunner.ts` around lines 201 - 226, The public method is
named GetStatus (PascalCase) but should follow the repo camelCase convention;
rename MigrationRunner.GetStatus to getStatus and update all internal references
(including the static call sites and any tests) to use getStatus, keeping the
implementation and return type identical and ensuring references to
this.Migrations and EnsureMigrationsTable remain correct.

src/Middleware/QueueReadyMiddleware.ts (1)

22-38: ⚡ Quick win

Rename the exported middleware to camelCase.

RequireQueueReady adds a new PascalCase function name to the module API. requireQueueReady would match the repo’s TypeScript naming convention.

As per coding guidelines, **/*.{ts,tsx}: Use camelCase for function names and variable names in TypeScript.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Middleware/QueueReadyMiddleware.ts` around lines 22 - 38, The exported
middleware function is named in PascalCase (RequireQueueReady) but should be
camelCase; rename the function to requireQueueReady and update the default
export to export default requireQueueReady, keeping the internal logic (calls to
QueueService.GetInstance().IsReady(), Logger.Warn,
ResponseHelper.ServiceUnavailable, UNAVAILABLE_MESSAGE) unchanged; also
search-and-replace any imports/usages of RequireQueueReady to requireQueueReady
so consumers compile and tests pass.

src/Controllers/WorkspaceController.ts (1)

54-162: ⚡ Quick win

Keep the new handler names in camelCase.

The new controller surface uses PascalCase method names (List, Create, AssignProjectWorkspace, etc.). Please align new TypeScript function names with the repo’s camelCase convention.

As per coding guidelines, **/*.{ts,tsx}: Use camelCase for function names and variable names in TypeScript.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/WorkspaceController.ts` around lines 54 - 162, The controller
methods are using PascalCase; rename them to camelCase to match project
conventions: change WorkspaceController.List -> list, Create -> create, Update
-> update, Delete -> delete, and AssignProjectWorkspace ->
assignProjectWorkspace (locations: the class WorkspaceController method
definitions and any external usages such as route handlers or tests that call
these methods). Update all references to these symbols (e.g., new
WorkspaceController().List -> .list) and ensure exported default remains
unchanged.

src/Controllers/ProjectTemplateController.ts (1)

50-168: ⚡ Quick win

Use camelCase for the new controller handlers.

The new public methods are introduced as List, GetById, Create, Update, and Delete. Please keep new TypeScript function names in camelCase to match the repository convention.

As per coding guidelines, **/*.{ts,tsx}: Use camelCase for function names and variable names in TypeScript.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/ProjectTemplateController.ts` around lines 50 - 168, Rename
the controller methods to camelCase to follow project conventions: change
ProjectTemplateController.List -> list, GetById -> getById, Create -> create,
Update -> update, and Delete -> delete; update all internal method definitions
in the class (the async handlers currently named List, GetById, Create, Update,
Delete) and ensure all external usages (route bindings, tests, or any callers
that reference ProjectTemplateController methods) are updated to the new names
so nothing breaks; keep all other logic (parseId usage, TemplateImmutableError
handling, ResponseHelper and Logger calls, and the Service property) unchanged.

src/Migrations/012_add_queue_job_id_to_deployments.ts (1)

10-13: ⚡ Quick win

Rename migration-scoped variables to camelCase.

TABLE, COLUMN, and INDEX violate the project’s TypeScript naming rule for variables.

As per coding guidelines **/*.{ts,tsx}: Use camelCase for function names and variable names.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Migrations/012_add_queue_job_id_to_deployments.ts` around lines 10 - 13,
Rename the migration-scoped constants TABLE, COLUMN, and INDEX to camelCase
(e.g., table, column, index) and update every reference in this migration file
(including inside up/down functions and any SQL/knex calls) to use the new
names; ensure any created index or column names embedded in strings are updated
to the new variable identifiers so the migration logic remains identical but
variable names follow the project's camelCase rule.

src/Migrations/999_migrate_pending_deployments.ts (1)

22-23: Gate merge on green CI for this migration-heavy release.

Given the number of schema + queue-path changes, require full CI (typecheck/lint/tests/coverage) to pass before merge.

Based on learnings: Require successful completion of GitHub Actions CI pipeline (typecheck, lint, test, coverage gates) before merging code.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Migrations/999_migrate_pending_deployments.ts` around lines 22 - 23,
Enable a "merge on green" CI gate for this release by configuring repository
branch protection and documentation: in GitHub branch protection for the target
branch require pull requests, enable "Require status checks to pass before
merging" and add the workflow job names for typecheck, lint, test, and coverage
(match the exact job names used by your GitHub Actions), enable "Require
branches to be up to date before merging", and ensure administrators cannot
bypass; also update CONTRIBUTING.md or the PR template referencing this
migration file (999_migrate_pending_deployments.ts) and its up(queryInterface)
change to state that merges require all CI checks (typecheck, lint, tests,
coverage) to pass.

src/Models/Project.ts (1)

160-167: ⚡ Quick win

Add an index for WorkspaceId to support workspace-scoped project queries.

The new foreign key is nullable and will likely be used in filters/joins; indexing it avoids avoidable scans as Projects grows.

Proposed diff

   indexes: [
     {
       name: 'idx_projects_name',
       unique: true,
       fields: ['Name'],
     },
     {
       name: 'idx_projects_is_active',
       fields: ['IsActive'],
     },
+    {
+      name: 'idx_projects_workspace_id',
+      fields: ['WorkspaceId'],
+    },
   ],

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Models/Project.ts` around lines 160 - 167, Add a DB index for the new
nullable foreign key to avoid table scans: update the Project model (the
WorkspaceId column definition) to include a model-level index on ['WorkspaceId']
(e.g., add an indexes array to the Project model config referencing
'WorkspaceId'), and create a corresponding migration that adds the index on the
Projects table (and a down migration to drop it) so existing databases get the
new index.

src/Models/ProjectNotificationSubscription.ts (1)

58-58: ⚡ Quick win

Validate Events at the model boundary.

DataTypes.JSON accepts any shape; add a validator to enforce non-empty array + enum membership so malformed events don’t get persisted.

Proposed diff

-    Events: { type: DataTypes.JSON, allowNull: false, field: 'Events' },
+    Events: {
+      type: DataTypes.JSON,
+      allowNull: false,
+      field: 'Events',
+      validate: {
+        isValidEvents(value: unknown) {
+          if (!Array.isArray(value) || value.length === 0) {
+            throw new Error('Events must be a non-empty array');
+          }
+          const allowed = new Set(Object.values(ENotificationEvent));
+          if (!value.every((event) => allowed.has(event as ENotificationEvent))) {
+            throw new Error('Events contains unsupported notification event(s)');
+          }
+        },
+      },
+    },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Models/ProjectNotificationSubscription.ts` at line 58, The Events
attribute on the ProjectNotificationSubscription model currently uses
DataTypes.JSON without validation; add a validator on the Events field (in
ProjectNotificationSubscription) that ensures the value is an array, not empty,
and that every item is one of the allowed event names (use the project's
canonical event enum/constant or inline allowed list), throwing a validation
error if any check fails so malformed event payloads cannot be persisted.

src/Routes/EnvironmentVariableRoutes.ts (1)

27-27: ⚡ Quick win

Rename InitializeRoutes to camelCase.

Use initializeRoutes to match the TypeScript naming rule for function names.

As per coding guidelines **/*.{ts,tsx}: Use camelCase for function names and variable names.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Routes/EnvironmentVariableRoutes.ts` at line 27, Rename the method
InitializeRoutes to camelCase initializeRoutes in the EnvironmentVariableRoutes
class: update the method declaration name from InitializeRoutes to
initializeRoutes, update every internal call site or override that references
InitializeRoutes (e.g., any constructors or lifecycle setup code that invokes
this method), and adjust any tests or exports that reference the old symbol so
all references use initializeRoutes consistently; ensure TypeScript compiles and
no stray references to InitializeRoutes remain.

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes